[SPARK-34699][SQL] 'CREATE OR REPLACE TEMP VIEW USING' should uncache correctly #31825
Conversation
| // Replacing with a different relation. The cache should be cleared. | ||
| sql(s"CREATE OR REPLACE $tempViewStr ${ident.table} USING parquet OPTIONS (path '$path2')") | ||
| assert(!spark.catalog.isCached(viewName)) | ||
| assert(spark.sharedState.cacheManager.isEmpty) |
There was a problem hiding this comment.
The existing code fails here because the temp view is not uncached.
There was a problem hiding this comment.
It's a bit fragile to test cacheManager as it's global.
Can we create a new temp view with path1 and check if it's cached?
|
cc @MaxGekk @sunchao @cloud-fan FYI |
|
Test build #136034 has finished for PR 31825 at commit
|
|
retest this please |
|
shall also we clean up the code in |
|
Kubernetes integration test starting |
|
Kubernetes integration test status failure |
|
Test build #136062 has finished for PR 31825 at commit
|
|
Good catch on the cache issue! Unrelated: do we have documentation for the |
Do you mean removing this condition? If so, I am planning to address it with SPARK-34700, in which we can change the signature of |
|
Test build #136076 has started for PR 31825 at commit |
|
@imback82 let's leave it to your follow-up work. |
|
retest this please |
|
thanks, merging to master! |
…d take/return more concrete types ### What changes were proposed in this pull request? Now that all the temporary views are wrapped with `TemporaryViewRelation`(#31273, #31652, and #31825), this PR proposes to update `SessionCatalog`'s APIs for temporary views to take or return more concrete types. APIs that will take `TemporaryViewRelation` instead of `LogicalPlan`: ``` createTempView, createGlobalTempView, alterTempViewDefinition ``` APIs that will return `TemporaryViewRelation` instead of `LogicalPlan`: ``` getRawTempView, getRawGlobalTempView ``` APIs that will return `View` instead of `LogicalPlan`: ``` getTempView, getGlobalTempView, lookupTempView ``` ### Why are the changes needed? Internal refactoring to work with more concrete types. ### Does this PR introduce _any_ user-facing change? No, this is internal refactoring. ### How was this patch tested? Updated existing tests affected by the refactoring. Closes #31906 from imback82/use_temporary_view_relation. Authored-by: Terry Kim <[email protected]> Signed-off-by: Wenchen Fan <[email protected]>
What changes were proposed in this pull request?
This PR proposes:
CREATE OR REPLACE TEMP VIEW USINGshould useTemporaryViewRelationto store temp views.Why are the changes needed?
This is a part of an ongoing work to wrap all the temporary views with
TemporaryViewRelation: SPARK-34698.This also fixes a bug where the temp view being replaced is not uncached.
Does this PR introduce any user-facing change?
Yes, the temp view being replaced with
CREATE OR REPLACE TEMP VIEW USINGis correctly uncached if the temp view is cached.How was this patch tested?
Added new tests.